Skip to content

Conversation

@HackerFoo
Copy link
Contributor

Objective

Implements #3567

Solution

This propagates the flag to the 3d render pass, which selects between LoadOp::Load and LoadOp::Clear for the first pass. This builds on #3412 and #3552.

@github-actions github-actions bot added the S-Needs-Triage This issue needs to be labelled label Jan 6, 2022
@alice-i-cecile alice-i-cecile added A-Rendering Drawing game state to the screen C-Feature A new feature, making something new possible and removed S-Needs-Triage This issue needs to be labelled labels Jan 7, 2022
Copy link
Member

@aevyrie aevyrie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm an absolute novice when it comes to rendering, so I can't provide much intelligent feedback, with the exception of the public facing API.

..Default::default()
})
.insert(FirstPassCube)
.insert(first_pass_layer);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very similar to an API suggestion I made on discord, however it sounds like cart would prefer a slightly different direction: https://discord.com/channels/691052431525675048/931614194229477417/931974884534415380

In short, instead of adding a render layer component on rendered entities, the suggestion was that we should do the reverse - register the components we want to render in a pass, with that render pass. This leaves the open question of how to unregister entities from the main pass, because by default we want all 3d things to render into the main pass. The suggested answer was that we wrap PbrBundles in a Mesh3d type, and register Mesh3d with the main pass. This way, if you want to render your mesh in another pass, you would make a PbrBundle and add your MyUiMesh component to it.

In cart's words:

Gotcha. yeah feel free to experiment. I would also like to consider paths that don't need that extra abstraction. Ex: a Hud entity will be a "hud thing" by nature of having Hud components. A normal "3d mesh" entity will be a "main pass" thing by nature of having normal 3d mesh components.

Personally, I'm not convinced cart's suggestion as I understand it is a better route. I prefer something like what you've done here, in which case we might add a MainPass component to PbrBundle or a wrapper type - which is what we had in 0.5, the MainPass component was removed unintentionally.

One suggestion I'd like you to consider, regardless of the outcome of the above, is how plugins might interoperate. How could 3d HUD plugins compose if they all want to draw on top of the main pass? Is this something the user would be responsible for configuring manually? Could a render pass component be passed in as a plugin constructor argument?

Copy link
Contributor Author

@HackerFoo HackerFoo Jan 18, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The API isn't great, but I'm mostly using the existing API (including RenderLayers, which I like.) I considered something like adding a Z-order to the Camera, but then composing cameras would be difficult, because you really want something more like the render graph, so you can express relations like "after the main pass, but before the UI". So I think it might be best to just rely on the render graph, and improve that API if needed e.g. avoid strings.

The issue of layering has come up when using both egui and Bevy's UI, because it's strange to make bevy_egui depend on bevy_ui just to declare that egui's pass should happen afterwards. Although the app should probably make that decision, anyway.

So this PR is just to make layering possible, and reasonably convenient, but I expect the usability could be improved in later PRs.

@superdump
Copy link
Contributor

@HackerFoo I assume this can be closed now that camera-driven rendering was merged?

@HackerFoo
Copy link
Contributor Author

Probably.

@superdump
Copy link
Contributor

Closing. Reopen if necessary. :)

@superdump superdump closed this Jun 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Rendering Drawing game state to the screen C-Feature A new feature, making something new possible

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants